[coverage] Conformance findings: AUTH-013 (#909) - #912
Open
peco-engineer-bot[bot] wants to merge 2 commits into
Open
[coverage] Conformance findings: AUTH-013 (#909)#912peco-engineer-bot[bot] wants to merge 2 commits into
peco-engineer-bot[bot] wants to merge 2 commits into
Conversation
![peco-engineer-bot[bot]](https://github.com/ghassets/images/gravatars/gravatar-user-420.png){kind=avatar}
Signed-off-by: peco-engineer-bot[bot] <3815206+peco-engineer-bot[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Verdict: 1 Medium · 1 Low
Targeted AUTH-013 fix that correctly forwards caller scopes/port for custom client_ids and adds two red→green tests. One medium concern: the refactor also changed the default-client_id path to honor oauth_redirect_port (and custom scopes) even without a caller oauth_client_id, which is untested and contradicts the PR's own stated principle — worth confirming this widening is intentional.
Addresses: - #3798853448 at src/databricks/sql/auth/auth.py:141 - #3798853453 at src/databricks/sql/auth/auth.py:128 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Automated fix for #909 — [coverage] Conformance findings: AUTH-013.
Fixed get_python_sql_connector_auth_provider in src/databricks/sql/auth/auth.py to honour a caller-supplied U2M OAuth bundle: it now forwards the caller's oauth_scopes verbatim (instead of hardcoding PYSQL_OAUTH_SCOPES) and, when a foreign oauth_client_id is supplied without a redirect port, falls through to the app-neutral base default [8030] rather than pinning the driver's own app-specific range (8020–8024). This is an offline-only, client-side computed artifact (resolved redirect port/scopes at provider construction) anchored in the AUTH-013 shared spec + reference PR databricks-sql-kernel#247; the interactive U2M login is never completed so the behavior is not end-to-end observable, making the unit test the correct verification. Both red→green tests and the full auth unit suite (15 passed) are green.
Root cause & plan
Root cause: In
get_python_sql_connector_auth_provider(src/databricks/sql/auth/auth.py, ~lines 115–134), a caller-supplied U2M OAuth bundle is not honored when a customoauth_client_idis given: (A)oauth_scopesis hardcoded toPYSQL_OAUTH_SCOPESand the caller'soauth_scopeskwarg is never read, so custom scopes like["all-apis"]are silently replaced by the driver defaultsql offline_access; (B) the redirect port only honorsoauth_redirect_portwhen paired withoauth_client_id, otherwise falling back to the driver's OWN app-specificPYSQL_OAUTH_REDIRECT_PORT_RANGE(8020–8024) — so a foreign client_id with no port gets pinned to the driver's default app port 8020, producing redirect_uri_mismatch (PECOBLR-4039) instead of falling through to the base default (8030). The spec principle: a caller who supplies their own client_id owns the rest of the bundle; the driver must not pin its own app-specific defaults.Files:
src/databricks/sql/auth/auth.py,tests/unit/test_auth.pyPlanned coverage:
Files changed
tests/unit/test_auth.pysrc/databricks/sql/auth/auth.pyTest plan
tests/unit/test_auth.py::Auth::test_get_python_sql_connector_u2m_explicit_bundle_override— fails (red) against the original code, passes (green) after the fixtests/unit/test_auth.py::Auth::test_get_python_sql_connector_u2m_foreign_client_id_no_port— fails (red) against the original code, passes (green) after the fix🤖 Generated by engineer-bot (bug-fix flow) — review before merge.